Skip to content

Conversation

@andrewspode
Copy link

In the original code, if there was an error doing the long poll, it would wait 100ms and then try again. If it errors again, it doubles to 200ms. If it errors again, it doubles again etc. with no cap.

This means after 10 errors in a row, you will be in a position of waiting a minute and a half before it attempts to reconnect. After that, 3 minutes and then it frankly spirals out of control!

That means if you lose internet connectivity (entirely possible if your wireless/3g is flakey) - you will very quickly, essentially lose your polling abilities.

My suggestion is to put a cap on this timeout - I've simply added in a conditional so that it will never go above 5 seconds. In my own use case, I'm actually going to keep it at 100ms and not multiply, so it might be worth having this as an option instead of hard coded.

In the original code, if there was an error doing the long poll, it would wait 100ms and then try again. If it errors again, it doubles to 200ms. If it errors again, it doubles again etc. with no cap. 

This means after 10 errors in a row, you will be in a position of waiting a minute and a half before it attempts to reconnect. After that, 3 minutes and then it frankly spirals out of control!

That means if you lose internet connectivity (entirely possible if your wireless/3g is flakey) - you will very quickly, essentially lose your polling abilities.

My suggestion is to put a cap on this timeout - I've simply added in a conditional so that it will never go above 5 seconds. In my own use case, I'm actually going to keep it at 100ms and not multiply, so it might be worth having this as an option instead of hard coded.
@tilgovi
Copy link
Contributor

tilgovi commented May 17, 2013

I think 5s is too low. We can be much smarter about this whole thing, IMO.

What do you think of

  • Backoff 2x as before
  • Limit the backoff only to cases where the connection timed out. Losing connectivity should mean total, fast failure in many cases, and we could detect the difference between these cases. In the case of server error or no connectivity, that error should bubble immediately and the application can decide what to do (either sleep, back off, or hook into the connectivity notification APIs that are cropping up in modern browsers)
  • Add a random jitter to the back-off. When timeouts are happening because the server is overwhelmed it could help to prevent clients from stampeding. Such a jitter is a classic feature of backoffs like this.

Finally, I haven't looked at this code in a while, but we should be sure that the whole thing is cancellable by the application in case the backoff gets large.

@andrewspode
Copy link
Author

Agreed. Rather selfishly, for my own uses I just want it to keep trying rather than back off at all. So this is more of a "are you aware of this issue" rather than an actual proposed fix.

I'm not sure who is looking after this plugin - but I'm happy to code these additions if it would be appreciated, otherwise the current maintainer might want to look at the issue.

@nomicode
Copy link
Contributor

The whole community maintains it, so feel free to submit another changeset!

@tilgovi
Copy link
Contributor

tilgovi commented Jun 1, 2013

@unclespode Cool. Thanks for the pull request. I'd love to see the fuller solution (distinguish timeouts from fast, total connection failure), but it's not critical. If you get the time to improve this please put it here. As is, though, I'm ambivalent about what is the better default.

@andrewspode
Copy link
Author

I've been working on the code as I develop my project at the moment. I've actually written EventSource (SSE) support into the changes function, so that it prefers this over longpolling, with a fallback to long polling where it isn't available, with no discernible difference as far as the coder is concerned.

I won't submit the patch until it's finalised.

kocolosk added a commit that referenced this pull request Jun 29, 2013
@wohali
Copy link
Member

wohali commented Mar 19, 2017

jquery.couch.js has been moved to https://github.com/apache/couchdb-jquery-couch . If you are still keen on addressing this issue, and the issue still exists, please file a new PR on that repository.

@wohali wohali closed this Mar 19, 2017
nickva pushed a commit to cloudant/couchdb that referenced this pull request Apr 21, 2017
This closes apache#60

Signed-off-by: ILYA Khlopotov <iilyak@ca.ibm.com>
nickva added a commit to cloudant/couchdb that referenced this pull request Apr 21, 2017
lag-linaro pushed a commit to lag-linaro/couchdb that referenced this pull request Oct 25, 2018
Add latest version getopt that automatically wraps usage output lines
jaydoane pushed a commit that referenced this pull request Dec 25, 2020
Update lager dep to 2.0.0rc2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants